improvement(client-presence): Json in-out improvements #24772

jason-ha · 2025-06-06T06:50:09Z

Externally:

reduce data in requirement to JsonSerializable as that is sufficient. See test(client-core-interfaces): JsonSerializable is enough #24751. Corresponding "local" events are updated to match.

Internally:

use OpaqueJsonDeserialized as stored and message type for generic (unknown) cases. system:presence workspace data values are kept exact under interface ConnectionValueState.
add helpers for needed conversions

Co-authored-by: Tyler Butler tylerbu@microsoft.com

Simplify internal unknown (generic) customer data that must be Json serializable using Opaque Json types At boundaries of API ins and outs, Json filtered types are cast to/from Opaque Json types. See new helpers: - `fullySerializableToOpaqueJson` - `asDeserializedJson` - `asDeeplyReadonlyDeserializedJson` Notifications utilities are updated and removal of `string &` for `NotificationListeners` allowed clean up of `@ts-ignore-error`. `NotificationsManagerImpl`'s `emit` implementation now just types `name` to `string`.'

…lizable as we are confident that is sufficient. See microsoft#24751. Update opaque cast helper to only require `JsonSerializable`.

Note: `testUtils.ts`'s `prepareConnectedPresence` has error where an `InboundClientJoinMessage` is supposed to satisfy `OutboundClientJoinMessage`.

after ignoring remaining type issue in presenceDatastoreManager.ts `PresenceDatastore` does not satisfy `DatastoreMessageContent`

- `PresenceDatastore` -> `DatastoreMessageContent` - `InboundClientJoinMessage` -> `OutboundClientJoinMessage`

…ities" This reverts commit 2aab49e.

- `serializableToOpaqueJson` -> `toOpaqueJson` - `asDeserializedJson` -> `revealOpaqueJson`

Copilot

Pull Request Overview

This PR improves JSON input/output handling for client-presence by simplifying the externally required data to JsonSerializable and switching internal handling to use OpaqueJsonDeserialized. In addition, the changes update tests and internal helpers to consistently wrap and unwrap JSON values using the new opaque JSON utilities.

Removed the redundant JsonDeserialized requirement in initial values.
Updated tests to wrap literal JSON objects with toOpaqueJson.
Refactored multiple components (including notifications, value managers, and internal utilities) to use opaque JSON conversion and reveal functions.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/framework/presence/src/test/presenceStates.spec.ts	Updated the createValueManager signature to remove the redundant JsonDeserialized.
packages/framework/presence/src/test/presenceDatastoreManager.spec.ts	Replaced raw JSON objects with toOpaqueJson wrappers in test cases.
packages/framework/presence/src/test/notificationsManager.spec.ts	Updated notification test values to use opaque JSON wrapping.
packages/framework/presence/src/test/latestValueManager.spec.ts	Added an extra test for inferred nullable types and adjusted opaque JSON usage.
packages/framework/presence/src/test/latestMapValueManager.spec.ts	Renamed a key from "true" to "boolean" and used toOpaqueJson consistently.
packages/framework/presence/src/test/batching.spec.ts	Replaced inline JSON values with toOpaqueJson wrappers for consistency.
packages/framework/presence/src/systemWorkspace.ts	Refactored state update handling with revealOpaqueJson for JSON unwrapping.
packages/framework/presence/src/notificationsManager.ts	Converted notification payload values using toOpaqueJson and revealOpaqueJson.
packages/framework/presence/src/latestValueManager.ts	Adjusted local value setter to wrap the value with toOpaqueJson and use a new shallow clone helper.
packages/framework/presence/src/latestMapValueManager.ts	Applied opaque JSON conversion in set and emit methods.
packages/framework/presence/src/internalUtils.ts	Introduced new helper functions (toOpaqueJson, revealOpaqueJson, asDeeplyReadonlyDeserializedJson) for opaque JSON conversion.
packages/framework/presence/api-report/*	Updated API types and documentation to reflect the new opaque JSON conventions.

Comments suppressed due to low confidence (4)

packages/framework/presence/src/test/latestMapValueManager.spec.ts:204

The key has been renamed from 'true' to 'boolean'; verify that no consumer of this test data relies on the previous key naming to avoid breaking changes.

boolean: true,

packages/framework/presence/src/notificationsManager.ts:247

The use of revealOpaqueJson to unwrap the notification update payload is a key design change. Please ensure that the structure of the unwrapped value matches the expected notification format throughout the system.

const value = revealOpaqueJson(updateValue.value);

packages/framework/presence/src/test/presenceStates.spec.ts:46

The change to remove JsonDeserialized from the initial value type is clear; please ensure that downstream code expects only JsonSerializable without relying on deserialized data.

initial: JsonSerializable<T>,

packages/framework/presence/src/latestValueManager.ts:128

Using toOpaqueJson to wrap the value in the local setter enforces opaque JSON conversion. Confirm that this change aligns with the expected handling of JSON values in all related components.

this.value.value = toOpaqueJson<T>(value);

packages/framework/presence/src/systemWorkspace.ts

tylerbutler

Just a couple of docs nits. Thanks for pulling this together.

Josmithr · 2025-06-09T17:04:55Z

packages/framework/presence/src/exposedInternalTypes.ts

@@ -26,17 +23,35 @@ export namespace InternalTypes {
 	}

 	/**
+	 * `ValueRequiredState` is used to represent a state that may have a value.


Wrong type used here. I also generally recommend against repeating the name of the item in its docs.

Suggested change

* `ValueRequiredState` is used to represent a state that may have a value.

* Represents a state that may have a value.

Using the name is continued trial of addressing the style/format question from end of May. (Awaiting yours and @WayneFerrao's comment) See last message: #24718 (comment)
I think we are likely to get the name wrong like this; so I think we don't have it. Full sentences? Use period even if not?

Replied in the other PR. Sorry, never saw that comment.

packages/framework/presence/src/exposedInternalTypes.ts

packages/framework/presence/src/exposedUtilityTypes.ts

packages/framework/presence/src/latestValueManager.ts

Josmithr

Left some nitpicks, but overall looks good to me.

github-actions · 2025-06-09T18:50:02Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  222981 links
    1707 destination URLs
    1939 URLs ignored
       0 warnings
       0 errors

jason-ha added 12 commits June 2, 2025 15:07

improvement(client-presence): reduce data in requirement to JsonSeria…

a5a0947

…lizable as we are confident that is sufficient. See microsoft#24751. Update opaque cast helper to only require `JsonSerializable`.

test(client-presence): updates for Opaque Json use

37eb489

Note: `testUtils.ts`'s `prepareConnectedPresence` has error where an `InboundClientJoinMessage` is supposed to satisfy `OutboundClientJoinMessage`.

docs(client-presence): update API reports

90bf2a2

after ignoring remaining type issue in presenceDatastoreManager.ts `PresenceDatastore` does not satisfy `DatastoreMessageContent`

merge: 'main' into presence/use-opaque-json

08ff94d

test(client-presence): correction after merge

24537dd

fix(client-presence): workaround "system:presence" incompatilities

2aab49e

- `PresenceDatastore` -> `DatastoreMessageContent` - `InboundClientJoinMessage` -> `OutboundClientJoinMessage`

Revert "fix(client-presence): workaround "system:presence" incompatil…

6d8059f

…ities" This reverts commit 2aab49e.

fix(client-presence): remove opacity from system datastore

667aebc

merge: 'main' into presence/use-opaque-json

6ffdcb4

docs(client-presence): comment additions

c619299

refactor(client-presence): rename helpers

cad419d

- `serializableToOpaqueJson` -> `toOpaqueJson` - `asDeserializedJson` -> `revealOpaqueJson`

Copilot AI review requested due to automatic review settings June 6, 2025 06:50

jason-ha requested a review from a team as a code owner June 6, 2025 06:50

jason-ha requested review from tylerbutler and WillieHabi and removed request for Copilot June 6, 2025 06:50

github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct public api change Changes to a public API base: main PRs targeted against main branch labels Jun 6, 2025

Copilot AI reviewed Jun 6, 2025

View reviewed changes

revert(client-presence): unneeded code changes

b6da44a

jason-ha force-pushed the presence/use-opaque-json branch from 200091e to b6da44a Compare June 6, 2025 07:24

tylerbutler reviewed Jun 7, 2025

View reviewed changes

packages/framework/presence/src/systemWorkspace.ts Outdated Show resolved Hide resolved

tylerbutler reviewed Jun 7, 2025

View reviewed changes

packages/framework/presence/src/systemWorkspace.ts Outdated Show resolved Hide resolved

tylerbutler approved these changes Jun 7, 2025

View reviewed changes

tylerbutler mentioned this pull request Jun 7, 2025

refactor(presence): Use branded JsonDeserialized type internally #24641

Closed

jason-ha added 2 commits June 7, 2025 14:40

docs(client-presence): typo fixes

8d3150b

merge: 'main' into presence/use-opaque-json

50265d0

tylerbutler approved these changes Jun 8, 2025

View reviewed changes

Josmithr reviewed Jun 9, 2025

View reviewed changes

packages/framework/presence/src/exposedInternalTypes.ts Outdated Show resolved Hide resolved

Josmithr reviewed Jun 9, 2025

View reviewed changes

packages/framework/presence/src/exposedUtilityTypes.ts Outdated Show resolved Hide resolved

Josmithr reviewed Jun 9, 2025

View reviewed changes

packages/framework/presence/src/latestValueManager.ts Outdated Show resolved Hide resolved

Josmithr reviewed Jun 9, 2025

View reviewed changes

packages/framework/presence/src/latestValueManager.ts Show resolved Hide resolved

Josmithr approved these changes Jun 9, 2025

View reviewed changes

docs(client-presence): comment improvements

d326c10

jason-ha enabled auto-merge (squash) June 9, 2025 20:09

tylerbutler approved these changes Jun 9, 2025

View reviewed changes

jason-ha merged commit 03152a6 into microsoft:main Jun 9, 2025
35 checks passed

jason-ha deleted the presence/use-opaque-json branch June 9, 2025 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

improvement(client-presence): Json in-out improvements #24772

improvement(client-presence): Json in-out improvements #24772

Uh oh!

jason-ha commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

tylerbutler left a comment

Uh oh!

Josmithr Jun 9, 2025

Uh oh!

jason-ha Jun 9, 2025

Uh oh!

Josmithr Jun 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Josmithr left a comment

Uh oh!

github-actions bot commented Jun 9, 2025

Uh oh!

Uh oh!

Uh oh!

	* `ValueRequiredState` is used to represent a state that may have a value.
	* Represents a state that may have a value.

improvement(client-presence): Json in-out improvements #24772

improvement(client-presence): Json in-out improvements #24772

Uh oh!

Conversation

jason-ha commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

tylerbutler left a comment

Choose a reason for hiding this comment

Uh oh!

Josmithr Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

jason-ha Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Josmithr Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Josmithr left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 9, 2025

linkcheck output

Uh oh!

Uh oh!

Uh oh!